Skip to content

West Midlands | 26-Jan-ITP | Fida Ali Zada | Sprint 2 | Data Group#985

Open
alizada-dev wants to merge 17 commits intoCodeYourFuture:mainfrom
alizada-dev:coursework/sprint-2
Open

West Midlands | 26-Jan-ITP | Fida Ali Zada | Sprint 2 | Data Group#985
alizada-dev wants to merge 17 commits intoCodeYourFuture:mainfrom
alizada-dev:coursework/sprint-2

Conversation

@alizada-dev
Copy link
Copy Markdown

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

Changelist

Completed the exercises in the Sprint-2 folder

@alizada-dev alizada-dev added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 9, 2026
// Then it should return false or throw an error

test("contains on invalid parameters returns false", () => {
expect(contains(["a", "b", "c"])).toStrictEqual(false);
Copy link
Copy Markdown
Contributor

@cjyuan cjyuan Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does your function return the value you expect from the following function calls?

contains(["a", "b", "c"], "1")
contains("abc", "1")
contains(null, "1")
contains(undefined, "1")

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now, it handles correctly.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test cannot not yet confirm that the function correctly returns false when the first argument is an array.
This is because contains(["a", "b", "c"]) is the executed as contains("[a", "b", "c"], undefined), which could also return false simply because "undefined" is not a key of the array.

Arrays are objects, with their indices acting as keys. A proper test should use a valid
key
to ensure the function returns false specifically because the input is an array, not because the key is missing.

Currently your function will return true in these two function calls.

  • contains(["a", "b", "c"], "1")
  • contains("abc", "1")

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now, I understand what you are telling, sir. I changed the function first:
`function contains(obj, x) {
if (
typeof obj !== "object" ||
obj === null ||
Array.isArray(obj)
) {
return false;
}

return Object.prototype.hasOwnProperty.call(obj, x);

}

module.exports = contains;`

And the test:
test("contains() handles arrays, strings, null, and undefined safely", () => { expect(contains([["a", "b", "c"], "1"])).toBe(false); expect(contains(["abc", "1"])).toBe(false); })

Please check if it is okay; otherwise, I will change.

return Object.fromEntries(sortedArray);
}

console.log(countWords("you? and! me, and you. me me me me hello Me"));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does your function return what you expect in the following function calls?

countWords("Hello,World! Hello World!");
countWords("constructor constructor");
countWords("          Hello World      ");

Note: The spec is not clear about exactly what to expect from these function calls. This is just for self-check.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for pointing out the bugs. Now, this should work for all cases.

`function countWords(str) {
const words = str
.toLowerCase()
.replace(/[^a-zA-Z0-9\s]/g, '') // replace punctuation with space
.trim()
.split(/\s+/); // split on any white space

const countObj = Object.create(null);

for (const word of words) {
countObj[word] = (countObj[word] || 0) + 1;
}

// Object.entries() converts object to array
const sortedArray = Object.entries(countObj).sort((a, b) => b[1] - a[1]);

// Object.fromEntries() coverts the sorted array to object
return Object.fromEntries(sortedArray);
}`

Comment on lines +27 to +28
// a) What is the target output when totalTill is called with the till object
// £NaN
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the question is asking for the "expected output", not the actual output.

Copy link
Copy Markdown
Author

@alizada-dev alizada-dev Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// a) What is the target output when totalTill is called with the till object
// £4.40

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Mar 20, 2026
@alizada-dev
Copy link
Copy Markdown
Author

I am sorry, sir, for responding late. I got a job because it was so stressful not working.

@alizada-dev alizada-dev added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Apr 2, 2026
@cjyuan
Copy link
Copy Markdown
Contributor

cjyuan commented Apr 3, 2026

The proposed changes look good. Have you applied the changes and pushed the changes to GitHub? I don't see any new commits on this PR branch.

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Apr 3, 2026
@alizada-dev
Copy link
Copy Markdown
Author

I had forgotten to push the changes. I just pushed them.

@alizada-dev alizada-dev added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Apr 4, 2026
@cjyuan
Copy link
Copy Markdown
Contributor

cjyuan commented Apr 4, 2026

Still nothing new on this branch. Did you push the changes on the right branch?

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Apr 4, 2026
@alizada-dev alizada-dev added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Apr 6, 2026
@cjyuan
Copy link
Copy Markdown
Contributor

cjyuan commented Apr 6, 2026

Those commits were done on your local computer. I cannot see them unless you sync/push them to GitHub.

It is a good practice to double check what you have committed to this PR branch on GitHub by clicking the "Files changed" tab to review them.

image

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Apr 6, 2026
@alizada-dev
Copy link
Copy Markdown
Author

Dear Sir, I am confused. I have committed and pushed all the changes to this PR branch on GitHub. If you check my latest commits, they are shown in the "Files changed" tab as well.

@cjyuan
Copy link
Copy Markdown
Contributor

cjyuan commented Apr 11, 2026

Sorry. I noticed no changes were made to till.js and count-words.js, and thought you didn't push your commits to GitHub.

I have now marked the addressed comments as "Resolved conversation".

Can you addressed the remaining three unresolved conversation?

@alizada-dev alizada-dev added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Apr 11, 2026
@alizada-dev
Copy link
Copy Markdown
Author

It is all right, sir. Thank you for your time and comments. I brought some changes. If the changes look good, I will push them to GitHub.

@cjyuan
Copy link
Copy Markdown
Contributor

cjyuan commented Apr 11, 2026

The proposed changes look good. I will mark this PR as completed.

Note: Sharing code as a "code block" in PR comment can make the code easier to read.

@cjyuan cjyuan added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Apr 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants